Skip to content

MSD: Add sidebar top padding when no logo#109880

Merged
p-jackson merged 1 commit intotrunkfrom
ciab-msd-sidebar-top-logo
Apr 8, 2026
Merged

MSD: Add sidebar top padding when no logo#109880
p-jackson merged 1 commit intotrunkfrom
ciab-msd-sidebar-top-logo

Conversation

@p-jackson
Copy link
Copy Markdown
Member

@p-jackson p-jackson commented Apr 8, 2026 •
edited
Loading

Fixes DOTMSD-1206

CleanShot 2026-04-08 at 15 01 51@2x

Proposed Changes

  • Add padding-block-start: 24px to .dashboard-responsive-sidebar__sidebar when it has no __logo child, via :not(:has(...)).

Why are these changes being made?

CIAB (my.woo.ai) boots the dashboard w/ Logo: null. With dashboard/omnibar on, the dashboard's own <Header> is also skipped, so the first sidebar menu item rendered flush under the external omnibar — no breathing room. Dotcom is unaffected (always has a Logo).

CIAB may or may not eventually have a logo here, but while MSD supports it we should have some sort of way to handle having no logo.

Testing Instructions

  • Enable dashboard/omnibar in config/dashboard-development.json.
  • Load CIAB w/ omnibar on — confirm ~24px padding above "Stores" item.
  • Load dotcom — confirm logo block still renders, no extra top padding stacked.
  • Toggle omnibar off on CIAB — confirm no regression under own header.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • Have you used memoizing on expensive computations?
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)?
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026 •
edited by matticbot
Loading

Calypso Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-175061
Jetpack Cloud Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-175061&env=jetpack
Automattic for Agencies Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-175061&env=a8c-for-agencies
Dashboard Live (dotcom) (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-175061&env=dashboard
Dashboard Live (CIAB) (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-175061&env=dashboard-ciab

@p-jackson p-jackson self-assigned this Apr 8, 2026
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Apr 8, 2026 •
edited
Loading

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • help-center
  • notifications
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug ciab-msd-sidebar-top-logo on your sandbox.

p-jackson
p-jackson commented Apr 8, 2026
View reviewed changes
client/dashboard/app/responsive-sidebar/sidebar.scss
@@ -20,6 +20,10 @@
overflow-y: auto;
}

.dashboard-responsive-sidebar__sidebar:not( :has( > .dashboard-responsive-sidebar__logo ) ) {
padding-block-start: $grid-unit-30;
Copy link
Copy Markdown
Member Author

@p-jackson p-jackson Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this padding number from the existing flex gap value used between the horizontal rule and the nav items.

Image

But it does mean that it's not even: the top and left/right paddings are different.

Image

Not blocking I don't think, but do you have an opinion about how much padding to use between the omnibar and the first nav items when there's no logo @lucasmendes-design?

@p-jackson p-jackson marked this pull request as ready for review April 8, 2026 03:17
@p-jackson p-jackson requested a review from a team as a code owner April 8, 2026 03:17
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 8, 2026
@StevenDufresne
Copy link
Copy Markdown
Contributor

StevenDufresne commented Apr 8, 2026

Code is fine. Looks like you ran into the intermittent build problem on calypso.live. Rebasing should fix.

@p-jackson
MSD: Add sidebar top padding when no logo
62df34c 
CIAB passes Logo: null; with dashboard/omnibar on, the menu sat flush
under the omnibar. Use :has() to apply 24px top padding when the logo
element isn't rendered.
@p-jackson p-jackson force-pushed the ciab-msd-sidebar-top-logo branch from 671a15f to 62df34c Compare April 8, 2026 05:09
fushar
fushar approved these changes Apr 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@fushar fushar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think it's fine to merge this to avoid CIAB folks from seeing broken layout 🙈

@p-jackson p-jackson merged commit 116c385 into trunk Apr 8, 2026
13 checks passed
@p-jackson p-jackson deleted the ciab-msd-sidebar-top-logo branch April 8, 2026 23:07
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fushar fushar fushar approved these changes

Assignees

@p-jackson p-jackson

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@p-jackson @matticbot @StevenDufresne @fushar